-
Notifications
You must be signed in to change notification settings - Fork 1
fix: missing acl schema #888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
based on the assumption that clusterDomainSuffix is always present in settings |
ferruhcihan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, the approach looks good 👍 I just noticed that some tests are failing in workloadUtils.test.ts, and there’s a small improvement needed in the URL checks instead of exact match.
| post: | ||
| operationId: getWorkloadCatalog | ||
| x-eov-operation-handler: v1/workloadCatalog | ||
| x-aclSchema: Workload |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why schema is called workload if the resource is tWorkloadCatalog?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WorkloadCatalog is part of the Workload workflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean we inherit the same ACL from Workload ? If so please add a comment. Otherwise it looks like a mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses bug APL-1412 by adding missing ACL schema declarations and replacing the isGiteaURL helper function with a direct string comparison against the cluster domain suffix.
- Adds
x-aclSchema: Workloadto three API endpoints that were missing ACL schema declarations - Replaces
isGiteaURL()function calls with explicit URL comparison usingclusterDomainSuffixparameter - Threads the
clusterDomainSuffixparameter through the workload utility functions
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/openapi/api.yaml | Adds missing x-aclSchema: Workload declarations to workload catalog endpoints |
| src/utils/workloadUtils.ts | Updates URL validation from isGiteaURL() to explicit domain suffix comparison |
| src/otomi-stack.ts | Passes cluster?.domainSuffix to workload functions and normalizes import ordering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
also fixes: APL-1422 |
tracks this bug: https://track.akamai.com/jira/browse/APL-1412
APL-1422 should also be fixed with this